-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/building simple send #16868
Feat/building simple send #16868
Conversation
da50bdc
to
d350cfb
Compare
Jenkins BuildsClick to see older builds (92)
|
59b4155
to
9f2a213
Compare
c52e770
to
89daa99
Compare
9f2a213
to
51fc469
Compare
2a789a7
to
93224cb
Compare
56dffde
to
017cf6c
Compare
Screen.Recording.2024-12-10.at.9.24.52.AM.movI think it now works the way you mentioned @benjthayer ;)
Fixed, I bring in the sticky header after a bit of padding.
I cant see it in design, but I tried some playing with FastBlur and it looks like this Screen.Recording.2024-12-09.at.10.45.24.PM.mov
Fixed in this task #16834 |
98aeb7e
to
16e67a2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! 👍
0272273
to
8c89690
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Massive work! It's looking great!
Just some minor findings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need the animation on fees change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the fees change happens automatically from the go side and is instantaneous, in the design Ben had mentioned if instantaneous, loading state is not needed but I cannot find a link right now :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm referring to the new animation we have on swap and dapps. When the fee value changes, there is a nice animation where the text blinks a few times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm Ive not seen this! @benjthayer should it be shown here and can you please point me to the design please?
8c89690
to
e0a6422
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Massive work :) overall looks good to me, some suggestions inline
e0a6422
to
1259e60
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for now, pending TODOs will be done in followup PRs
1259e60
to
b6919a3
Compare
b6919a3
to
a40b6e6
Compare
Unfortunately this PR most likely caused this rather hard to catch regression (as hinted by this subtle console warning):
Notice the dialog lost the the "aspect preview" combobox; compare to: |
A
Aaah oh no. I can work on the fix or are you already on it @caybro ? |
fixes #16836
What does the PR do
This PR add some dummy components + some actual ones in order to fix a mechanism for the dialog sizing.
Please not even though the AccountSelector, TokenSelector and NetworkFilter are added in this PR they are not worked on completely yet and is still something that is WIP under Epic #16696
Please note this PR doesn't cover QML Units tests, I will work on them next.
The Recipient selector is only temporary one and not done yet TBD under #16916
Affected areas
SimpleSend.qml
Architecture compliance
My PR is consistent with this document: Status Desktop Architecture Guide
Screenshot of functionality (including design for comparison)
Screen.Recording.2024-12-04.at.7.14.49.PM.mov
Screen.Recording.2024-12-04.at.7.19.35.PM.mov
Screen.Recording.2024-12-08.at.11.21.18.PM.mov